LCORE-1859: Fix llama-stack startup as non-root user#1859
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🧰 Additional context used📓 Path-based instructions (3)tests/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/app/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (1)📚 Learning: 2026-04-06T20:18:07.852ZApplied to files:
🔇 Additional comments (2)
WalkthroughMount RAG seeds read-only and add entrypoint copy-on-start seeding for lightspeed-stack and llama-stack. Protect post-stream topic-summary generation with a try/except to log failures without aborting an already-started SSE response; add a unit test ensuring the stream completes and results are stored with topic_summary=None on failure. ChangesRAG Seed Mounting and Entrypoint Seeding
Post-stream Topic Summary Error Containment
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker-compose-library.yaml`:
- Around line 26-28: The SELinux mount label casing is inconsistent between the
two volume entries ("./tests/e2e/rag:/opt/app-root/rag-seed:ro,Z" vs
"./scripts/lightspeed-stack-entrypoint.sh:/app-root/lightspeed-stack-entrypoint.sh:ro,z");
pick one casing and make both mounts consistent (either change the RAG seed
mount to use :ro,z or the entrypoint script to :ro,Z) unless the differing
semantics (private vs shared) are intentional—update the line containing
"./tests/e2e/rag:/opt/app-root/rag-seed:ro,Z" or the line containing
"./scripts/lightspeed-stack-entrypoint.sh:/app-root/lightspeed-stack-entrypoint.sh:ro,z"
accordingly.
In `@scripts/lightspeed-stack-entrypoint.sh`:
- Around line 16-20: When $RAG_SEED_DIR exists, check whether any .db files
actually exist before running cp to avoid the cryptic "cannot stat" error; test
the glob (e.g. set -- "$RAG_SEED_DIR"/*.db and verify [ -e "$1" ] or loop to
detect files) and if none are found, emit a clear error/warning mentioning
RAG_SEED_DIR and skip or exit instead of running cp into STORAGE_RAG_DIR; update
the block referencing RAG_SEED_DIR and STORAGE_RAG_DIR and the cp invocation
accordingly.
In `@scripts/llama-stack-entrypoint.sh`:
- Around line 18-24: The RAG seeding block in scripts/llama-stack-entrypoint.sh
is duplicated in scripts/lightspeed-stack-entrypoint.sh; extract that logic into
a shared function (e.g., seed_rag_kvstore) in a new script (e.g.,
scripts/seed-rag-kvstore.sh) and then source and call seed_rag_kvstore from both
entrypoint scripts to remove duplication; ensure the function uses the same
environment variables (RAG_SEED_DIR, STORAGE_RAG_DIR), checks that the seed
directory exists and contains .db files before copying, emits the same echo
messages, and returns non-zero on fatal errors so both llama-stack-entrypoint.sh
and lightspeed-stack-entrypoint.sh simply call seed_rag_kvstore to perform the
seeding.
- Around line 20-24: The seed-copy block that checks RAG_SEED_DIR and runs cp
may fail with a cryptic error when there are no *.db files; update the shell
snippet that uses RAG_SEED_DIR and STORAGE_RAG_DIR to validate that at least one
.db file exists before running cp, e.g., test the glob (for f in
"$RAG_SEED_DIR"/*.db; do [ -e "$f" ] && break; done) and if no files found print
a clear error/warning and skip or exit, otherwise proceed to mkdir -p
"$STORAGE_RAG_DIR" and cp the files; ensure the message mentions the missing .db
files and uses the same variable names (RAG_SEED_DIR, STORAGE_RAG_DIR) so it’s
easy to locate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9e16fb92-3b33-4064-8f0d-df37304d1c14
📒 Files selected for processing (4)
docker-compose-library.yamldocker-compose.yamlscripts/lightspeed-stack-entrypoint.shscripts/llama-stack-entrypoint.sh
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E Tests for Lightspeed Evaluation job
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-05-20T08:09:36.724Z
Learnt from: max-svistunov
Repo: lightspeed-core/lightspeed-stack PR: 1580
File: src/client.py:104-108
Timestamp: 2026-05-20T08:09:36.724Z
Learning: In the lightspeed-stack repo, the synthesized `run.yaml` file handling in `src/client.py` (`_synthesize_library_config`) uses a fixed `/tmp` path intentionally in the PoC (PR `#1580`). The durable production requirements are tracked in spec doc R10 (docs/design/llama-stack-config-merge/llama-stack-config-merge.md): persistent known path overwritten each boot, file mode 0600 set via explicit create flag (not umask), and a `--synthesized-config-output` CLI flag for debugging. The PoC code is scheduled for removal pre-merge; the implementation JIRA "Unified llama_stack.config schema + synthesizer" inherits R10's requirements.
Applied to files:
docker-compose-library.yamldocker-compose.yamlscripts/llama-stack-entrypoint.sh
📚 Learning: 2025-08-18T10:56:55.349Z
Learnt from: matysek
Repo: lightspeed-core/lightspeed-stack PR: 292
File: pyproject.toml:0-0
Timestamp: 2025-08-18T10:56:55.349Z
Learning: The lightspeed-stack project intentionally uses a "generic image" approach, bundling many dependencies directly in the base runtime image to work for everyone, rather than using lean base images with optional dependency groups.
Applied to files:
docker-compose-library.yaml
📚 Learning: 2026-05-20T08:09:30.641Z
Learnt from: max-svistunov
Repo: lightspeed-core/lightspeed-stack PR: 1580
File: docs/design/llama-stack-config-merge/poc-results/library-mode/synthesized-run.yaml:107-110
Timestamp: 2026-05-20T08:09:30.641Z
Learning: In Llama-stack config YAMLs, when defining a Llama Guard safety shield entry, set `provider_shield_id` to the *guard model identifier* (e.g., `meta-llama/Llama-Guard-3-8B`). Do not use a chat/generative model id (e.g., `openai/gpt-4o-mini`): a chat-model id (or `native_override`) indicates only an override landed and does **not** mean the safety shield is actually gating queries. Ensure any E2E coverage for the related implementation (JIRA/E2E tests) exercises a real Llama Guard model to verify that the shield is effective.
Applied to files:
docker-compose-library.yamldocker-compose.yaml
📚 Learning: 2026-05-12T15:14:34.788Z
Learnt from: syedriko
Repo: lightspeed-core/lightspeed-stack PR: 1727
File: scripts/konflux_requirements.sh:9-15
Timestamp: 2026-05-12T15:14:34.788Z
Learning: In this repo, the `.konflux/` directory is committed/tracked and is guaranteed to exist in a fresh clone. Therefore, shell scripts that write output under `.konflux/` (e.g., create files like `.konflux/<...>`) should not waste effort by calling `mkdir -p .konflux` first. Only add directory-creation logic if the script may run in an environment/repo state where `.konflux/` might not be present.
Applied to files:
scripts/lightspeed-stack-entrypoint.shscripts/llama-stack-entrypoint.sh
🔇 Additional comments (3)
docker-compose.yaml (1)
22-25: LGTM!scripts/lightspeed-stack-entrypoint.sh (1)
22-25: LGTM!docker-compose-library.yaml (1)
23-26: RAG storage permissions for UID 1001 are already set in the image
deploy/lightspeed-stack/Containerfilecreates/opt/app-root/src/.llama/storageandchown -R 1001:1001 /opt/app-root/src/.llama, and the image setsUSER 1001; this makes/opt/app-root/src/.llama/storage/ragwritable in filesystem-backed library mode.
| - ./tests/e2e/rag:/opt/app-root/rag-seed:ro,Z | ||
| # Host copy so `docker compose up` picks up script changes without rebuilding | ||
| - ./scripts/lightspeed-stack-entrypoint.sh:/app-root/lightspeed-stack-entrypoint.sh:ro,z |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Inconsistent SELinux label casing between mounts.
The RAG seed mount uses :ro,Z (private label) while the entrypoint script mount uses :ro,z (shared label). For consistency and clarity, consider using the same casing unless the different labeling is intentional.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose-library.yaml` around lines 26 - 28, The SELinux mount label
casing is inconsistent between the two volume entries
("./tests/e2e/rag:/opt/app-root/rag-seed:ro,Z" vs
"./scripts/lightspeed-stack-entrypoint.sh:/app-root/lightspeed-stack-entrypoint.sh:ro,z");
pick one casing and make both mounts consistent (either change the RAG seed
mount to use :ro,z or the entrypoint script to :ro,Z) unless the differing
semantics (private vs shared) are intentional—update the line containing
"./tests/e2e/rag:/opt/app-root/rag-seed:ro,Z" or the line containing
"./scripts/lightspeed-stack-entrypoint.sh:/app-root/lightspeed-stack-entrypoint.sh:ro,z"
accordingly.
| if [ -d "$RAG_SEED_DIR" ]; then | ||
| echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..." | ||
| mkdir -p "$STORAGE_RAG_DIR" | ||
| cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/ | ||
| fi |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Improve error handling for missing seed files.
If $RAG_SEED_DIR exists but contains no *.db files, the glob pattern will fail to expand and cp will exit with a cryptic error (cp: cannot stat '.../*.db': No such file or directory). Consider adding explicit validation to provide a clearer error message.
🛡️ Proposed fix to add validation
if [ -d "$RAG_SEED_DIR" ]; then
echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..."
mkdir -p "$STORAGE_RAG_DIR"
+ if ! compgen -G "$RAG_SEED_DIR/*.db" > /dev/null; then
+ echo "ERROR: No .db files found in $RAG_SEED_DIR"
+ exit 1
+ fi
cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ -d "$RAG_SEED_DIR" ]; then | |
| echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..." | |
| mkdir -p "$STORAGE_RAG_DIR" | |
| cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/ | |
| fi | |
| if [ -d "$RAG_SEED_DIR" ]; then | |
| echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..." | |
| mkdir -p "$STORAGE_RAG_DIR" | |
| if ! compgen -G "$RAG_SEED_DIR/*.db" > /dev/null; then | |
| echo "ERROR: No .db files found in $RAG_SEED_DIR" | |
| exit 1 | |
| fi | |
| cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/ | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/lightspeed-stack-entrypoint.sh` around lines 16 - 20, When
$RAG_SEED_DIR exists, check whether any .db files actually exist before running
cp to avoid the cryptic "cannot stat" error; test the glob (e.g. set --
"$RAG_SEED_DIR"/*.db and verify [ -e "$1" ] or loop to detect files) and if none
are found, emit a clear error/warning mentioning RAG_SEED_DIR and skip or exit
instead of running cp into STORAGE_RAG_DIR; update the block referencing
RAG_SEED_DIR and STORAGE_RAG_DIR and the cp invocation accordingly.
| RAG_SEED_DIR="${RAG_SEED_DIR:-/opt/app-root/rag-seed}" | ||
| STORAGE_RAG_DIR="${STORAGE_RAG_DIR:-/opt/app-root/src/.llama/storage/rag}" | ||
| if [ -d "$RAG_SEED_DIR" ]; then | ||
| echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..." | ||
| mkdir -p "$STORAGE_RAG_DIR" | ||
| cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/ | ||
| fi |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Consider extracting duplicated seeding logic.
The RAG seeding block (lines 18-24) is identical to the logic in scripts/lightspeed-stack-entrypoint.sh (lines 14-20). Consider extracting this into a shared function or sourced script to reduce duplication and ensure consistency.
♻️ Example: Extract to a shared function
Create a shared script (e.g., scripts/seed-rag-kvstore.sh):
#!/bin/bash
# Shared function to seed RAG kvstore
seed_rag_kvstore() {
local RAG_SEED_DIR="${RAG_SEED_DIR:-/opt/app-root/rag-seed}"
local STORAGE_RAG_DIR="${STORAGE_RAG_DIR:-/opt/app-root/src/.llama/storage/rag}"
if [ -d "$RAG_SEED_DIR" ]; then
echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..."
mkdir -p "$STORAGE_RAG_DIR"
if ! compgen -G "$RAG_SEED_DIR/*.db" > /dev/null; then
echo "ERROR: No .db files found in $RAG_SEED_DIR"
exit 1
fi
cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/
fi
}Then source and call it in both entrypoint scripts:
source /app-root/seed-rag-kvstore.sh
seed_rag_kvstore🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/llama-stack-entrypoint.sh` around lines 18 - 24, The RAG seeding
block in scripts/llama-stack-entrypoint.sh is duplicated in
scripts/lightspeed-stack-entrypoint.sh; extract that logic into a shared
function (e.g., seed_rag_kvstore) in a new script (e.g.,
scripts/seed-rag-kvstore.sh) and then source and call seed_rag_kvstore from both
entrypoint scripts to remove duplication; ensure the function uses the same
environment variables (RAG_SEED_DIR, STORAGE_RAG_DIR), checks that the seed
directory exists and contains .db files before copying, emits the same echo
messages, and returns non-zero on fatal errors so both llama-stack-entrypoint.sh
and lightspeed-stack-entrypoint.sh simply call seed_rag_kvstore to perform the
seeding.
| if [ -d "$RAG_SEED_DIR" ]; then | ||
| echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..." | ||
| mkdir -p "$STORAGE_RAG_DIR" | ||
| cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/ | ||
| fi |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Improve error handling for missing seed files.
If $RAG_SEED_DIR exists but contains no *.db files, the glob pattern will fail to expand and cp will exit with a cryptic error. Consider adding explicit validation for a clearer error message (same issue as in lightspeed-stack-entrypoint.sh).
🛡️ Proposed fix to add validation
if [ -d "$RAG_SEED_DIR" ]; then
echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..."
mkdir -p "$STORAGE_RAG_DIR"
+ if ! compgen -G "$RAG_SEED_DIR/*.db" > /dev/null; then
+ echo "ERROR: No .db files found in $RAG_SEED_DIR"
+ exit 1
+ fi
cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ -d "$RAG_SEED_DIR" ]; then | |
| echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..." | |
| mkdir -p "$STORAGE_RAG_DIR" | |
| cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/ | |
| fi | |
| if [ -d "$RAG_SEED_DIR" ]; then | |
| echo "Seeding RAG kvstore from $RAG_SEED_DIR into $STORAGE_RAG_DIR..." | |
| mkdir -p "$STORAGE_RAG_DIR" | |
| if ! compgen -G "$RAG_SEED_DIR/*.db" > /dev/null; then | |
| echo "ERROR: No .db files found in $RAG_SEED_DIR" | |
| exit 1 | |
| fi | |
| cp -f "$RAG_SEED_DIR"/*.db "$STORAGE_RAG_DIR"/ | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/llama-stack-entrypoint.sh` around lines 20 - 24, The seed-copy block
that checks RAG_SEED_DIR and runs cp may fail with a cryptic error when there
are no *.db files; update the shell snippet that uses RAG_SEED_DIR and
STORAGE_RAG_DIR to validate that at least one .db file exists before running cp,
e.g., test the glob (for f in "$RAG_SEED_DIR"/*.db; do [ -e "$f" ] && break;
done) and if no files found print a clear error/warning and skip or exit,
otherwise proceed to mkdir -p "$STORAGE_RAG_DIR" and cp the files; ensure the
message mentions the missing .db files and uses the same variable names
(RAG_SEED_DIR, STORAGE_RAG_DIR) so it’s easy to locate.
CodeHex083
changed the title
1 participant
Fixes #124
Seed RAG kvstore is now delivered read-only and copied into the writable storage volume at startup, so the non-root container user (UID 1001) can write the registry kvstore it shares. Fixes the 'attempt to write a readonly database' crash. Covers both server and library compose modes.
Summary by CodeRabbit
Chores
Bug Fixes
Tests